-
-
Notifications
You must be signed in to change notification settings - Fork 438
refactoring: implement a proper dependency (.d) file parser #2972
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
afc0826 to
ce2aa2a
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2972 +/- ##
==========================================
+ Coverage 69.34% 69.40% +0.06%
==========================================
Files 241 242 +1
Lines 18590 18632 +42
==========================================
+ Hits 12891 12932 +41
- Misses 4503 4504 +1
Partials 1196 1196
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ce2aa2a to
18ae29d
Compare
18ae29d to
e1bd351
Compare
lucarin91
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely a step in the correct direction, but I would try to make the parser step cleaner and work directly on the io.Reader in streaming (if possible). Anyway, for my limited knowledge of this part of the cli I didn't find a big issues, so we could also proceed to merge this, and improve that later.
Please check if the PR fulfills these requirements
See how to contribute
before creating one)
our contributing guidelines
UPGRADING.mdhas been updated with a migration guide (for breaking changes)configuration.schema.jsonupdated if new parameters are added.What kind of change does this PR introduce?
Refactor the dep (.d) file parser to handle more cases, and refine the subroutine API.
What is the current behavior?
No changes.
What is the new behavior?
No changes.
Does this PR introduce a breaking change, and is titled accordingly?
No.
Other information